RDKEMW-22184 : fetch controller link key - #267
Conversation
There was a problem hiding this comment.
Pull request overview
Adds persistence/consolidation of RF4CE controller link keys by writing per-controller “info” files under /opt/secure/lib/rf4ce/<network-mac>/<controller-mac>/info, and removes the prior IARM API path for fetching link keys.
Changes:
- Fetches controller link key from HAL on successful validation end and writes it to an on-disk info file.
- Adds on-start consolidation of info files (remove stale entries, add missing entries) and deletion on unbind.
- Removes the controller link-key request plumbing (IARM call + network request handler/types).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/rf4ce/ctrlm_rf4ce_validation.cpp | Writes controller info file at end of successful bind/validation. |
| src/rf4ce/ctrlm_rf4ce_network.h | Declares new info-file management helpers; removes link-key request handler declaration. |
| src/rf4ce/ctrlm_rf4ce_network.cpp | Implements info file create/delete/consolidation and hooks them into unbind/init. |
| src/rf4ce/ctrlm_rf4ce_controller.cpp | No functional change (formatting-only). |
| src/ctrlm_rcu.h | Removes link-key queue message type and API declaration. |
| src/ctrlm_rcu.cpp | Removes implementation of controller link-key IARM request. |
| src/ctrlm_rcu_iarm.cpp | Unregisters/removes the controller link-key IARM handler. |
| src/ctrlm_network.h | Removes virtual link-key request handler from base network class. |
| src/ctrlm_network.cpp | Removes base implementation for link-key request handler. |
| include/ctrlm_ipc_rcu.h | Removes IARM call name and IPC struct for controller link-key. |
Suppressed comments (2)
src/rf4ce/ctrlm_rf4ce_network.cpp:4932
- This warning logs a directory entry name which is likely a controller MAC address. Mask it when PII masking is enabled to avoid leaking identifiers in logs.
XLOGD_WARN("Unexpected entry in network dir: <%s>, skipping", entry->d_name);
src/rf4ce/ctrlm_rf4ce_network.cpp:4894
- This log prints the full on-disk path which includes network/controller MAC addresses. Mask it when PII masking is enabled (consistent with the error logs in this function).
XLOGD_INFO("Deleting info file <%s>", file_path);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Suppressed comments (5)
src/rf4ce/ctrlm_rf4ce_network.cpp:4850
- The per-network and per-controller directories are created with 0755, which can expose network/controller MAC addresses via directory listing. Use more restrictive permissions (e.g., 0700) for these directories.
if(mkdir(network_dir, 0755) != 0 && errno != EEXIST) {
int errsv = errno;
XLOGD_ERROR("Failed to create network dir <%s> error <%s>", ctrlm_is_pii_mask_enabled() ? "***" : network_dir, strerror(errsv));
return;
}
src/rf4ce/ctrlm_rf4ce_network.cpp:4893
- This log prints the full on-disk path, which embeds network/controller MAC addresses. Other logs in this module respect PII masking, so this should be masked consistently.
XLOGD_INFO("Deleting info file <%s>", file_path);
src/rf4ce/ctrlm_rf4ce_network.cpp:4931
- This warning logs the raw directory entry name, which can contain MAC addresses. It should respect PII masking like the other log lines in this function.
XLOGD_WARN("Unexpected entry in network dir: <%s>, skipping", entry->d_name);
include/ctrlm_ipc_rcu.h:28
- Removing the public IARM call/struct for ControllerLinkKey is a breaking change for external consumers. If this API is still supported, consider keeping the definition as a deprecated/unsupported call (or bumping/communicating the API revision and providing a migration path).
#define CTRLM_RCU_IARM_CALL_CONTROLLER_STATUS "Rcu_ControllerStatus" ///< IARM Call to get controller information
#define CTRLM_RCU_IARM_CALL_RIB_REQUEST_GET "Rcu_RibRequestGet" ///< IARM Call to retrieves an attribute from the controller's RIB
#define CTRLM_RCU_IARM_CALL_RIB_REQUEST_SET "Rcu_RibRequestSet" ///< IARM Call to set an attribute in the controller's RIB
#define CTRLM_RCU_IARM_CALL_RF4CE_POLLING_ACTION "Rcu_Rf4cePollingAction" ///< IARM Call to Send Remote Heartbeat Response Polling Action
src/rf4ce/ctrlm_rf4ce_network.cpp:4916
- The comment says this set tracks controllers with a "valid info file", but the code only tracks whether a matching controller directory entry exists. Either validate the info file or adjust the comment to match the implementation.
// Track which bound controllers already have a valid info file
std::set<unsigned long long> found_on_disk;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (5)
src/rf4ce/ctrlm_rf4ce_network.cpp:4893
- The delete-path log prints the full on-disk path (includes network/controller identifiers). Other logs in this area honor PII masking, so this should be masked consistently when ctrlm_is_pii_mask_enabled() is true.
XLOGD_INFO("Deleting info file <%s>", file_path);
src/rf4ce/ctrlm_rf4ce_network.cpp:4836
- The controller-info data is written under /opt/secure; creating the parent dir with 0755 allows non-privileged users to list the directory if it doesn't already exist. Use restrictive permissions (e.g., 0700) for directories created by this component.
if(mkdir(CTRLM_RF4CE_CONTROLLER_INFO_LIB_DIR, 0755) != 0 && errno != EEXIST) {
include/ctrlm_ipc_rcu.h:29
- This change removes the public IARM call/IPC surface for fetching a controller link key. If there are external consumers of include/ctrlm_ipc_rcu.h, this is a breaking API change; consider keeping a stubbed call for backward compatibility and/or bumping CTRLM_RCU_IARM_BUS_API_REVISION and documenting the removal.
#define CTRLM_RCU_IARM_CALL_CONTROLLER_STATUS "Rcu_ControllerStatus" ///< IARM Call to get controller information
#define CTRLM_RCU_IARM_CALL_RIB_REQUEST_GET "Rcu_RibRequestGet" ///< IARM Call to retrieves an attribute from the controller's RIB
#define CTRLM_RCU_IARM_CALL_RIB_REQUEST_SET "Rcu_RibRequestSet" ///< IARM Call to set an attribute in the controller's RIB
#define CTRLM_RCU_IARM_CALL_RF4CE_POLLING_ACTION "Rcu_Rf4cePollingAction" ///< IARM Call to Send Remote Heartbeat Response Polling Action
#define CTRLM_RCU_IARM_BUS_API_REVISION (13) ///< Revision of the RCU IARM API
src/rf4ce/ctrlm_rf4ce_network.cpp:4931
- This warning logs the raw directory entry name, which contains a controller MAC. Consider masking it when PII masking is enabled to avoid leaking identifiers in logs.
XLOGD_WARN("Unexpected entry in network dir: <%s>, skipping", entry->d_name);
src/rf4ce/ctrlm_rf4ce_network.h:26
- is included here but doesn't appear to be used anywhere in this header; removing unused includes helps keep dependencies minimal.
#include <string>
#include <array>
#include <map>
#include <vector>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
src/rf4ce/ctrlm_rf4ce_network.cpp:4836
CTRLM_RF4CE_CONTROLLER_INFO_LIB_DIRis under/opt/securebut is created with mode 0755, which allows other users to list/execute under this directory if it didn’t already exist. Since this code stores controller keys under this tree, the parent directory should be created with restrictive permissions as well (or be provisioned by the image with the desired perms).
if(mkdir(CTRLM_RF4CE_CONTROLLER_INFO_LIB_DIR, 0755) != 0 && errno != EEXIST) {
src/rf4ce/ctrlm_rf4ce_network.cpp:4920
- If
opendir(network_dir)fails (e.g., EACCES), consolidation silently does nothing. Logging the failure (while respecting PII masking) will make key-file issues diagnosable in the field.
DIR *dir = opendir(network_dir);
if(dir != NULL) {
| #define CTRLM_RCU_IARM_CALL_CONTROLLER_STATUS "Rcu_ControllerStatus" ///< IARM Call to get controller information | ||
| #define CTRLM_RCU_IARM_CALL_CONTROLLER_LINK_KEY "Rcu_ControllerLinkKey" ///< IARM Call to get controller link key | ||
| #define CTRLM_RCU_IARM_CALL_RIB_REQUEST_GET "Rcu_RibRequestGet" ///< IARM Call to retrieves an attribute from the controller's RIB | ||
| #define CTRLM_RCU_IARM_CALL_RIB_REQUEST_SET "Rcu_RibRequestSet" ///< IARM Call to set an attribute in the controller's RIB | ||
| #define CTRLM_RCU_IARM_CALL_RF4CE_POLLING_ACTION "Rcu_Rf4cePollingAction" ///< IARM Call to Send Remote Heartbeat Response Polling Action |
No description provided.